Add a custom join_paths function to wrap the error

This helps us not unwrap() and rather propagate the error upwards.

Closes #488

diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs
index bd1531491139b5a06b52844e30ccf2af4ee18edd..b464c684f227fe6ef81886dfc6934aebc9d8b07c 100644 (file)
--- a/src/cargo/ops/cargo_run.rs
+++ b/src/cargo/ops/cargo_run.rs
@@ -48,7 +48,8 @@ pub fn run(manifest_path: &Path,
         Some(path) => path,
         None => exe,
     };
-    let process = compile.process(exe, &root).args(args).cwd(os::getcwd());
+    let process = try!(compile.process(exe, &root))
+                              .args(args).cwd(os::getcwd());
 
     try!(options.shell.status("Running", process.to_string()));
     Ok(process.exec().err())

diff --git a/src/cargo/ops/cargo_rustc/compilation.rs b/src/cargo/ops/cargo_rustc/compilation.rs
index ebeeefe1f7c544a742516dc80f115a7bfe55bf11..646c79a384b3f450d306ced2ee6085128a954bf2 100644 (file)
--- a/src/cargo/ops/cargo_rustc/compilation.rs
+++ b/src/cargo/ops/cargo_rustc/compilation.rs
@@ -1,10 +1,9 @@
 use std::collections::HashMap;
 use std::dynamic_lib::DynamicLibrary;
-use std::os;
 use semver::Version;
 
 use core::{PackageId, Package};
-use util;
+use util::{mod, CargoResult};
 
 /// A structure returning the result of a compilation.
 pub struct Compilation {
@@ -60,30 +59,31 @@ impl Compilation {
     /// The package argument is also used to configure environment variables as
     /// well as the working directory of the child process.
     pub fn process<T: ToCStr>(&self, cmd: T, pkg: &Package)
-                              -> util::ProcessBuilder {
+                              -> CargoResult<util::ProcessBuilder> {
         let mut search_path = DynamicLibrary::search_path();
         for dir in self.native_dirs.values() {
             search_path.push(dir.clone());
         }
         search_path.push(self.root_output.clone());
         search_path.push(self.deps_output.clone());
-        let search_path = os::join_paths(search_path.as_slice()).unwrap();
+        let search_path = try!(util::join_paths(search_path.as_slice(),
+                                                DynamicLibrary::envvar()));
         let mut cmd = util::process(cmd).env(DynamicLibrary::envvar(),
                                              Some(search_path.as_slice()));
         for (k, v) in self.extra_env.iter() {
             cmd = cmd.env(k.as_slice(), v.as_ref().map(|s| s.as_slice()));
         }
 
-        cmd.env("CARGO_MANIFEST_DIR", Some(pkg.get_manifest_path().dir_path()))
-           .env("CARGO_PKG_VERSION_MAJOR",
-                Some(pkg.get_version().major.to_string()))
-           .env("CARGO_PKG_VERSION_MINOR",
-                Some(pkg.get_version().minor.to_string()))
-           .env("CARGO_PKG_VERSION_PATCH",
-                Some(pkg.get_version().patch.to_string()))
-           .env("CARGO_PKG_VERSION_PRE",
-                pre_version_component(pkg.get_version()))
-           .cwd(pkg.get_root())
+        Ok(cmd.env("CARGO_MANIFEST_DIR", Some(pkg.get_manifest_path().dir_path()))
+              .env("CARGO_PKG_VERSION_MAJOR",
+                   Some(pkg.get_version().major.to_string()))
+              .env("CARGO_PKG_VERSION_MINOR",
+                  Some(pkg.get_version().minor.to_string()))
+              .env("CARGO_PKG_VERSION_PATCH",
+                   Some(pkg.get_version().patch.to_string()))
+              .env("CARGO_PKG_VERSION_PRE",
+                   pre_version_component(pkg.get_version()))
+              .cwd(pkg.get_root()))
     }
 }
 

diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs
index ee023be19fc9562aa46ed1bf081e903598f53e26..34f6d1aa02459b8423d71af915ac857b6bb9040f 100644 (file)
--- a/src/cargo/ops/cargo_rustc/mod.rs
+++ b/src/cargo/ops/cargo_rustc/mod.rs
@@ -2,11 +2,10 @@ use std::collections::HashSet;
 use std::dynamic_lib::DynamicLibrary;
 use std::io::{fs, USER_RWX};
 use std::io::fs::PathExtensions;
-use std::os;
 
 use core::{SourceMap, Package, PackageId, PackageSet, Target, Resolve};
 use util::{mod, CargoResult, ProcessBuilder, CargoError, human, caused_human};
-use util::{Require, Config, internal, ChainError, Fresh, profile};
+use util::{Require, Config, internal, ChainError, Fresh, profile, join_paths};
 
 use self::job::{Job, Work};
 use self::job_queue as jq;
@@ -228,7 +227,7 @@ fn compile_custom(pkg: &Package, cmd: &str,
     let layout = cx.layout(pkg, KindTarget);
     let output = layout.native(pkg);
     let old_output = layout.proxy().old_native(pkg);
-    let mut p = process(cmd.next().unwrap(), pkg, cx)
+    let mut p = try!(process(cmd.next().unwrap(), pkg, cx))
                      .env("OUT_DIR", Some(&output))
                      .env("DEPS_DIR", Some(&output))
                      .env("TARGET", Some(cx.target_triple()))
@@ -309,7 +308,7 @@ fn rustc(package: &Package, target: &Target,
 fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>,
                  cx: &Context, req: PlatformRequirement)
                  -> CargoResult<Vec<(ProcessBuilder, Kind)>> {
-    let base = process("rustc", package, cx);
+    let base = try!(process("rustc", package, cx));
     let base = build_base_args(cx, base, package, target, crate_types.as_slice());
 
     let target_cmd = build_plugin_args(base.clone(), cx, package, target, KindTarget);
@@ -335,7 +334,7 @@ fn rustdoc(package: &Package, target: &Target,
     let kind = KindTarget;
     let pkg_root = package.get_root();
     let cx_root = cx.layout(package, kind).proxy().dest().join("doc");
-    let rustdoc = process("rustdoc", package, cx).cwd(pkg_root.clone());
+    let rustdoc = try!(process("rustdoc", package, cx)).cwd(pkg_root.clone());
     let mut rustdoc = rustdoc.arg(target.get_src_path())
                          .arg("-o").arg(cx_root)
                          .arg("--crate-name").arg(target.get_name());
@@ -545,7 +544,8 @@ fn build_deps_args(mut cmd: ProcessBuilder, target: &Target, package: &Package,
     }
 }
 
-pub fn process<T: ToCStr>(cmd: T, pkg: &Package, cx: &Context) -> ProcessBuilder {
+pub fn process<T: ToCStr>(cmd: T, pkg: &Package,
+                          cx: &Context) -> CargoResult<ProcessBuilder> {
     // When invoking a tool, we need the *host* deps directory in the dynamic
     // library search path for plugins and such which have dynamic dependencies.
     let layout = cx.layout(pkg, KindPlugin);
@@ -567,9 +567,10 @@ pub fn process<T: ToCStr>(cmd: T, pkg: &Package, cx: &Context) -> ProcessBuilder
 
     // We want to use the same environment and such as normal processes, but we
     // want to override the dylib search path with the one we just calculated.
-    let search_path = os::join_paths(search_path.as_slice()).unwrap();
-    cx.compilation.process(cmd, pkg)
-                  .env(DynamicLibrary::envvar(), Some(search_path.as_slice()))
+    let search_path = try!(join_paths(search_path.as_slice(),
+                                      DynamicLibrary::envvar()));
+    Ok(try!(cx.compilation.process(cmd, pkg))
+              .env(DynamicLibrary::envvar(), Some(search_path.as_slice())))
 }
 
 fn each_dep<'a>(pkg: &Package, cx: &'a Context, f: |&'a Package|) {

diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs
index 426fe9f804f415af96f2c0356ca76fe832e19e91..3e8de8282bb8b159b513d91525e6e78947f1e660 100644 (file)
--- a/src/cargo/ops/cargo_test.rs
+++ b/src/cargo/ops/cargo_test.rs
@@ -32,7 +32,7 @@ pub fn run_tests(manifest_path: &Path,
             Some(path) => path,
             None => exe.clone(),
         };
-        let cmd = compile.process(exe, &compile.package).args(test_args);
+        let cmd = try!(compile.process(exe, &compile.package)).args(test_args);
         try!(options.compile_opts.shell.concise(|shell| {
             shell.status("Running", to_display.display().to_string())
         }));
@@ -58,7 +58,7 @@ pub fn run_tests(manifest_path: &Path,
 
     for (lib, name) in libs {
         try!(options.compile_opts.shell.status("Doc-tests", name));
-        let mut p = compile.process("rustdoc", &compile.package)
+        let mut p = try!(compile.process("rustdoc", &compile.package))
                            .arg("--test").arg(lib)
                            .arg("--crate-name").arg(name)
                            .arg("-L").arg(&compile.root_output)

diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs
index 72fd9516fddee0e44cacd956bba00684c04cd761..e23ab7dc01fefc79d85afe8de081e6559da1caf7 100644 (file)
--- a/src/cargo/util/mod.rs
+++ b/src/cargo/util/mod.rs
@@ -4,7 +4,7 @@ pub use self::result::{Wrap, Require};
 pub use self::errors::{CargoResult, CargoError, BoxError, ChainError, CliResult};
 pub use self::errors::{CliError, FromError, ProcessError};
 pub use self::errors::{process_error, internal_error, internal, human, caused_human};
-pub use self::paths::realpath;
+pub use self::paths::{realpath, join_paths};
 pub use self::hex::{to_hex, short_hash};
 pub use self::pool::TaskPool;
 pub use self::dependency_queue::{DependencyQueue, Fresh, Dirty, Freshness};

diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs
index bfdf0ca90465604d1bcd2577a3d2761f463644de..28441e4fda58a495eb68a66c4c829f63166411bc 100644 (file)
--- a/src/cargo/util/paths.rs
+++ b/src/cargo/util/paths.rs
@@ -1,5 +1,8 @@
 use std::{io,os};
 use std::io::fs;
+use std::path::BytesContainer;
+
+use util::{human, CargoResult};
 
 pub fn realpath(original: &Path) -> io::IoResult<Path> {
     static MAX_LINKS_FOLLOWED: uint = 256;
@@ -38,3 +41,10 @@ pub fn realpath(original: &Path) -> io::IoResult<Path> {
     return Ok(result);
 }
 
+pub fn join_paths<T: BytesContainer>(paths: &[T], env: &str)
+                                     -> CargoResult<Vec<u8>> {
+    os::join_paths(paths).map_err(|e| {
+        human(format!("failed to join search paths together: {}\n\
+                       Does ${} have an unterminated quote character?", e, env))
+    })
+}